DOC: clarify add_reference_channels usage (Takeover #13664)#13846
DOC: clarify add_reference_channels usage (Takeover #13664)#13846Dpereaptkhamur-13 wants to merge 29 commits intomne-tools:mainfrom
Conversation
|
Hey @Dpereaptkhamur-13 , I believe you don't have an account on circle ci that's why those checks are failing , login into your account & those tests will pass ! |
|
Hi @HansujaB, thank you for the tip! I have now logged into CircleCI and authorized the account. The tests should be ready to run once a maintainer approves the workflow. Thanks again for the help! |
|
Hi @Dpereaptkhamur-13, thanks for opening the PR. Right now, the message has been added to Rather, the note should be included in Just make sure if you use this text to action the suggested changes: https://github.com/mne-tools/mne-python/pull/13664/changes#r2892847499 and https://github.com/mne-tools/mne-python/pull/13664/changes#r2892851172 Then, it would be good to also add a note to the |
|
Hi @tsbinns, thank you for the detailed feedback! That makes complete sense—moving the clarification to set_eeg_reference and the tutorials ensures it's visible to users regardless of which function they check first. I will update mne/utils/docs.py and the preprocessing tutorial file accordingly. I'll ping you again once the changes are pushed! |
…ereaptkhamur-13/mne-python into doc-ref-channel-clarification
for more information, see https://pre-commit.ci
fix: correct typo in docs.py Co-authored-by: Thomas S. Binns <t.s.binns@outlook.com>
doc: reformat reference channel note for clarity Co-authored-by: Thomas S. Binns <t.s.binns@outlook.com>
|
I've updated the changelog filename to 13846.other.rst and included the :newcontrib: tag with my name. I also alphabetized the bibliography and removed the extra blank lines in channels.py. Everything should be ready for a final review! |
tsbinns
left a comment
There was a problem hiding this comment.
@Dpereaptkhamur-13 Thanks for the changes, but there were some comments which were not adressed. Please address these now, as well as some final additional things.
Add notes to :func:`~mne.set_eeg_reference` and :ref:`_tut-set-eeg-ref` about the correct procedure to create an average reference, by :newcontrib:`Deep Kaur`. Co-authored-by: Thomas S. Binns <t.s.binns@outlook.com>
…ereaptkhamur-13/mne-python into doc-ref-channel-clarification
| @article{KimEtAl2023, | ||
| author = {Kim, Hyeonseok and Luo, Justin and Chu, Shannon and Cannard, Cedric and Hoffmann, Sven and Miyakoshi, Makoto}, | ||
| doi = {10.3389/frsip.2023.1064138}, | ||
| journal={Frontiers in Signal Processing}, | ||
| pages = {1064138}, | ||
| title={{ICA}’s bug: How ghost {IC}s emerge from effective rank deficiency caused by {EEG} electrode interpolation and incorrect re-referencing}, | ||
| volume = {3}, | ||
| year = {2023}, | ||
| } |
There was a problem hiding this comment.
@Dpereaptkhamur-13 I have just pushed some changes, including in 5d8f145 to add the correct information in the citation, as this had still not been addressed.
Reference issue (if any)
What does this implement/fix?
This PR completes the documentation clarification for add_reference_channels originally started in #13664. It addresses the mathematical requirements for average referencing in the docstrings, as requested in issue #13618.
Changes include:
Additional information